Skip to content

Conversation

@aw-was-here
Copy link
Contributor

@aw-was-here aw-was-here commented Apr 23, 2022

Also fixing various issues that prevent test-patch --version from working correctly. Woops.

Copy link
Member

@ndimiduk ndimiduk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, I guess. If you really want to. This seems like a path to bugs because maven doesn't support this natively. This is better than using the version plugin or something similar? Can we do anything else with this flatten plugin besides populate the version value? Would we need to?

@aw-was-here
Copy link
Contributor Author

aw-was-here commented Apr 25, 2022

This seems like a path to bugs because maven doesn't support this natively. This is better than using the version plugin or something similar? Can we do anything else with this flatten plugin besides populate the version value? Would we need to?

Yeah, it feels a bit ugly to me too, but I'm following the advice that the maven project themselves gave: https://maven.apache.org/maven-ci-friendly.html#install-deploy

It would be great if maven did this work without a plug-in, but if they are saying use flatten in the official docs, who am I to argue?

FWIW, there are a lot of wins for making this change:

  • Forks can actually set their own version just by doing mvn -Drevision=(foo)
  • The patch between versions is significantly smaller.
  • Rebasing old branches is much easier if they have to cross a version boundary.
  • The code to actually cut a release is much much easier to maintain.

@aw-was-here aw-was-here merged commit 0d82c90 into apache:main Apr 25, 2022
@aw-was-here aw-was-here deleted the mvn-experiment branch April 25, 2022 20:40
@aw-was-here
Copy link
Contributor Author

thanks for the review @ndimiduk !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants